Repository navigation
test: stabilise the Miniflare D1 suites (#190) - #192
Merged
Merged
Conversation
Both CI flakes in #190 were one Miniflare 4.20260730.0 bug: the synchronous binding proxy (`D1Database#prepare`/`bind`) waits on a shared 0/1 flag, so a late notify from the previous reply wakes the next call before its reply exists. The assert at fetch-sync.ts:147 fails and every later call reads the previous reply (the 1-3 ms cascade in roles.test.ts). The list-values "statement budget" failure has the same stack. Every async proxy call also went through dispatchFetch with undici's `reset`, one TIME_WAIT port per D1 query (2,000 queries left 2,002), which ran macOS out of ephemeral ports under parallel D1 files. No newer Miniflare 4.x exists, so patch the pin with the upstream fixes merged for Miniflare 5 only: workers-sdk#15552 (generation handshake) and #15781 (keep-alive dispatchFetch, 1 s idle), plus the same 1 s keep-alive on the sync proxy's pool (#16445 A). No retries, D1 files stay parallel. - patches/[email protected] via patchedDependencies - packages/storage/test/miniflare-patch.test.ts fails without the patch - .github/workflows/d1-stress.yml runs the Miniflare halves 20 times - ADR-0012 addendum, AGENTS.md note Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Madraka <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Madraka <[email protected]>
The #15781 backport made async proxy calls reuse sockets, but the timers that close idle ones run on the test's main thread. A stall past workerd's 5 s idle timeout then reuses a closed socket: ECONNRESET, not safely retryable (one local roles.test.ts run in 20; reproduced with a 5.5 s stall). Drop it and keep the shipped `reset`; the patch is now #15552 plus the sync pool's 1 s keep-alive, whose timers run on the proxy's own thread. The stress workflow adds one busy loop per vCPU: 20 unpatched runs at CI's own load passed, so it did not tell the builds apart. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Madraka <[email protected]>
The guard now forces the race on the real Miniflare: a `node -r` preload (Node loads it in worker threads too) stalls the proxy thread between its store and notify and the caller before its wait, on every other call. Unpatched, the first stalled call fails the CI assertion; patched, 300 prepare/bind calls stay in step. ADR-0012: record the stress results (unpatched also passes 20 CI runs without the forced stalls) and the local port exhaustion that stays open. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Madraka <[email protected]>
`fileURLToPath(new URL(...))` failed typecheck (DOM vs Node `URL`); build the paths with `path.join` like the other storage tests. ADR-0012 gains the recipe for regenerating the Miniflare patch on an upgrade. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Madraka <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #190.
Root cause
Both flakes in #190 are one upstream Miniflare 4.20260730.0 bug. Neither is a timing or statement-counting problem. The list-values "statement budget" failure (run 42789152142) ends in the same assertion as roles.test.ts (run 42793831919):
message?.id === idatminiflare/src/plugins/core/proxy/fetch-sync.ts:147. Both are reached fromD1Database#prepare/bind.Miniflare runs
prepare/bindas synchronous calls through a worker thread. The caller waits on a shared 0/1 flag. If the worker is preempted between itsAtomics.storeand itsAtomics.notify, and the caller has already read the reply, the late notify wakes the next call before its reply exists.receiveMessageOnPortreturnsundefinedand the assert throws. Every later call then reads the previous call's reply, which is the 1–3 ms cascade in roles.test.ts.Upstream fix: cloudflare/workers-sdk#15552, merged into Miniflare 5 only. 4.20260730.0 is still npm
latest, and no 4.x release carries the fix.Harness answers
undefinedon dispose.afterAll. No reuse after dispose.turbo --concurrency=2runs 2 packages at once. Vitest uses cores − 1 workers (3 on CI), or 2 for@cloudhal/api. That is up to 5 files at once, each with its own workerd and proxy thread, on 4 vCPUs. The load makes the preemption more likely, but the bug is the handshake.Fix
patches/[email protected](throughpatchedDependencies):packages/storage/test/miniflare-patch.test.tsplus two fixtures. Anode -rpreload, which Node also loads into worker threads, stalls both sides of the handshake on every other call. On real Miniflare, unpatched fails on the first stalled call with the CI assertion (3/3); patched completes 300 calls (3/3).About the Miniflare patch
Proof
CI stress (
.github/workflows/d1-stress.yml): runs the(Miniflare)halves of storage, core-ts, api, schedules, comments and analytics with CI'sturbo --concurrency=2. Each run is 10 jobs on Node 24 plus 10 on Node 26. It runs onworkflow_dispatchand on PRs that touch the patch.Plainly: the race is too rare to trigger on demand. It hit CI twice on 2026-11-10, yet 40 unpatched stress jobs passed. So the stress runs show no regression; they do not prove the fix. The proof is the forced reproduction above, which is now the guard test, and it matches the CI stack exactly.
Local runs, one process at a time, final patch:
@cloudhal/storage: 5/5@cloudhal/api: 5/5roles.test.ts: 20/20@cloudhal/core-ts: 0/5, every failureEADDRNOTAVAIL. This is the pre-existing local port problem below; the patch does not touch that code path.CI duration: on the final commit the TypeScript jobs took 19m11s (Node 24) and 12m06s (Node 26); an earlier commit took 18m08s / 18m26s. The last four green
mainruns took 12–18 min. Nothing is serialised.Not fixed here: local port exhaustion (older than this PR)
Async proxy calls go through
dispatchFetchwith undicireset: true. Each D1 query takes one TIME_WAIT port, which macOS holds for 30 s. One core-ts run fills 16,340 of 16,384 ephemeral ports, even at--maxWorkers=50%. Linux CI has not shown this.I backported workers-sdk#15781 (keep-alive) first. core-ts then passed 5/5 locally. But one roles.test.ts run in 20 failed with
ECONNRESETon two kept-alive sockets in a row. That can't be retried safely, because a write may already have run. A main-thread stall longer than 5 s reproduces it, but normal runs stall only about 0.5 s, so the cause is unsettled. That is a new CI risk, so I dropped the backport from this PR. The ADR records the details; it needs its own follow-up.🤖 Generated with Claude Code